Skip to content

fix: fail gracefully on private non pro #423

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 2 commits into from
May 12, 2023
Merged

fix: fail gracefully on private non pro #423

merged 2 commits into from
May 12, 2023

Conversation

RNR1
Copy link
Contributor

@RNR1 RNR1 commented May 11, 2023 •
edited
Loading

PR Checklist

Overview

Fail gracefully for branch protection settings for private repos (non-Pro).

  • Added warnText field to the "withSpinner" options
  • withSpinner will now store its callback return value, and will show warnText in case the value is false

Bonus: I changed all the catch (err) to catch (error) just for consistency (git blame on me)

I'm honestly not so excited about this solution, but submitting it anyway to hear your thoughts

@codecov
Copy link

codecov bot commented May 11, 2023

Codecov Report

Merging #423 (fd63b53) into main (424e108) will not change coverage.
The diff coverage is n/a.

❗ Current head fd63b53 differs from pull request most recent head 281368e. Consider uploading reports for the commit 281368e to get more accurate results

@@            Coverage Diff            @@
##              main      #423   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            1         1           
  Lines            5         5           
  Branches         3         3           
=========================================
  Hits             5         5

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

JoshuaKGoldberg
JoshuaKGoldberg reviewed May 12, 2023
View reviewed changes
script/setup.js
@@ -275,7 +275,7 @@ try {
let user;
try {
user = JSON.parse((await $`gh api user`).stdout).login;
} catch (err) {
} catch (error) {
Copy link
Owner

@JoshuaKGoldberg JoshuaKGoldberg May 12, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, heh, this is unused... +1 to calling it error over err. I suppose we should enable https://eslint.org/docs/latest/rules/no-unused-vars#caughterrors... I'll file a separate issue to do that as a followup.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#426

JoshuaKGoldberg
JoshuaKGoldberg approved these changes May 12, 2023
View reviewed changes
Copy link
Owner

@JoshuaKGoldberg JoshuaKGoldberg left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💯, this is excellent. Tried it out locally and it works great. Thanks @RNR1!

Returning false makes sense to me - we can always rethink later if it gets more convoluted over time.

Note that I filed #425 around curly styles as a followup. I prefer brackets always for consistency. So I'll do that myself 😄

@JoshuaKGoldberg JoshuaKGoldberg merged commit 488ba12 into JoshuaKGoldberg:main May 12, 2023
@github-actions
Copy link

🎉 This is included in version v1.27.80 🎉

The release is available on:

Cheers! 📦🚀

1 similar comment
@github-actions
Copy link

🎉 This is included in version v1.27.80 🎉

The release is available on:

Cheers! 📦🚀

@RNR1 RNR1 deleted the 401/fail-gracefully-on-private-non-pro branch May 13, 2023 05:56
@JoshuaKGoldberg
Copy link
Owner

@allcontributors add @RNR1 for design

@allcontributors allcontributors bot mentioned this pull request May 17, 2023
Merged
@allcontributors
Copy link
Contributor

@JoshuaKGoldberg

I've put up a pull request to add @RNR1! 🎉

JoshuaKGoldberg pushed a commit that referenced this pull request May 17, 2023
@allcontributors
docs: add RNR1 as a contributor for design (#443)
d984fe4 
Adds @RNR1 as a contributor for design.

This was requested by JoshuaKGoldberg [in this
comment](#423 (comment))

---------

Co-authored-by: allcontributors[bot] <46447321+allcontributors[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JoshuaKGoldberg JoshuaKGoldberg JoshuaKGoldberg approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

🚀 Feature: Fail setup more gracefully for branch protections on private, non-Pro repos
2 participants
@RNR1 @JoshuaKGoldberg